-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement package.json
's imports
field
#4895
Conversation
To share `exports`, `imports`, and `alias` remapping
The latest updates on your projects. Learn more about Vercel for Git ↗︎
10 Ignored Deployments
|
No dependency changes detected. Learn more about Socket for GitHub ↗︎ 👍 No new dependency issues detected in pull request Bot CommandsTo ignore an alert, reply with a comment starting with Pull request alert summary
|
✅ This change can build |
|
} | ||
} | ||
|
||
fn try_from(value: &Value, ty: ExportImport) -> Result<Self> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to reuse SubpathValue
parsing for imports
, I needed to pass an additional ExportImport
type to print the correct error messages. That prevents us from reusing the TryFrom
trait. The code is otherwise the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit worried about the name.
I think someone may expect SubpathValue::try_from
to be one from TryFrom
, and to use v.try_into()?
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to try_new
@@ -115,6 +115,53 @@ async fn base_resolve_options( | |||
let mut plugins = opt.plugins.clone(); | |||
plugins.push(UnsupportedSassResolvePluginVc::new(root).as_resolve_plugin()); | |||
|
|||
let conditions = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hoisted so that it can be shared for both "into" and "in" conditions. No changes.
crates/turbopack-tests/tests/snapshot/imports/subpath-imports/input/package.json
Show resolved
Hide resolved
Linux Benchmark for bd92efb
Click to view full benchmark
|
MacOS Benchmark for bd92efb
Click to view full benchmark
|
Linux Benchmark for 527498cClick to view benchmark
|
MacOS Benchmark for 527498c
Click to view full benchmark
|
vercel/turborepo#4895 renamed `ExportsValue` into `SubpathValue` --------- Co-authored-by: Tobias Koppers <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
vercel/turborepo#4895 renamed `ExportsValue` into `SubpathValue` --------- Co-authored-by: Tobias Koppers <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
### Description The [imports field](https://nodejs.org/api/packages.html#imports) is similar to `exports`. While `exports` allows a package to control how others can import it, `imports` allows it control how it wants to import other packages. The majority of the code can be shared with the already-implemented `ExportsField` and `AliasMap`, with just special parsing logic. Besides that, I just needed to setup the conditional for imports and some cleanup. ### Testing Instructions ```bash RUST_BACKTRACE=1 cargo nextest run -E 'test(snapshot__imports__subpath_imports)' ``` Fixes WEB-50 Pair: #49636 fix vercel/turborepo#4799 --------- Co-authored-by: Tobias Koppers <[email protected]>
### Description The [imports field](https://nodejs.org/api/packages.html#imports) is similar to `exports`. While `exports` allows a package to control how others can import it, `imports` allows it control how it wants to import other packages. The majority of the code can be shared with the already-implemented `ExportsField` and `AliasMap`, with just special parsing logic. Besides that, I just needed to setup the conditional for imports and some cleanup. ### Testing Instructions ```bash RUST_BACKTRACE=1 cargo nextest run -E 'test(snapshot__imports__subpath_imports)' ``` Fixes WEB-50 Pair: #49636 fix vercel/turborepo#4799 --------- Co-authored-by: Tobias Koppers <[email protected]>
### Description The [imports field](https://nodejs.org/api/packages.html#imports) is similar to `exports`. While `exports` allows a package to control how others can import it, `imports` allows it control how it wants to import other packages. The majority of the code can be shared with the already-implemented `ExportsField` and `AliasMap`, with just special parsing logic. Besides that, I just needed to setup the conditional for imports and some cleanup. ### Testing Instructions ```bash RUST_BACKTRACE=1 cargo nextest run -E 'test(snapshot__imports__subpath_imports)' ``` Fixes WEB-50 Pair: #49636 fix vercel/turborepo#4799 --------- Co-authored-by: Tobias Koppers <[email protected]>
Description
The imports field is similar to
exports
. Whileexports
allows a package to control how others can import it,imports
allows it control how it wants to import other packages.The majority of the code can be shared with the already-implemented
ExportsField
andAliasMap
, with just special parsing logic. Besides that, I just needed to setup the conditional for imports and some cleanup.Testing Instructions
RUST_BACKTRACE=1 cargo nextest run -E 'test(snapshot__imports__subpath_imports)'
Fixes WEB-50
Pair: vercel/next.js#49636
fix #4799